Skip to content

[SPARK-20814][mesos] Restore support for spark.executor.extraClassPath.#18037

Closed
vanzin wants to merge 2 commits intoapache:masterfrom
vanzin:SPARK-20814
Closed

[SPARK-20814][mesos] Restore support for spark.executor.extraClassPath.#18037
vanzin wants to merge 2 commits intoapache:masterfrom
vanzin:SPARK-20814

Conversation

@vanzin
Copy link
Contributor

@vanzin vanzin commented May 19, 2017

Restore code that was removed as part of SPARK-17979, but instead of
using the deprecated env variable name to propagate the class path, use
a new one.

Verified by running "./bin/spark-class o.a.s.executor.CoarseGrainedExecutorBackend"
manually.

[SPARK-20814][mesos] Restore support for spark.executor.extraClassPath.
d129327 
Restore code that was removed as part of SPARK-17979, but instead of
using the deprecated env variable name to propagate the class path, use
a new one.

Verified by running "./bin/spark-class o.a.s.executor.CoarseGrainedExecutorBackend"
manually.
@vanzin
Copy link
Contributor Author

vanzin commented May 19, 2017

@mgummelt

@SparkQA
Copy link

SparkQA commented May 19, 2017

Test build #77106 has finished for PR 18037 at commit d129327.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@SparkQA
Copy link

SparkQA commented May 20, 2017

Test build #77129 has finished for PR 18037 at commit a861819.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@vanzin
Copy link
Contributor Author

vanzin commented May 22, 2017

Ping? Let me invoke @srowen or @squito since we should have this in 2.2.0.

@mgummelt
Copy link

mgummelt commented May 22, 2017

looking...

@mgummelt
Copy link

mgummelt commented May 22, 2017

Was spark.executor.extraClassPath ever supported in Mesos? I see no code for it.

@vanzin
Copy link
Contributor Author

vanzin commented May 22, 2017

Yes, it just set SPARK_CLASSPATH and relied on the launcher code to respect that env variable, but that code was recently removed.

srowen
srowen approved these changes May 22, 2017
View reviewed changes
Copy link
Member

@srowen srowen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I trust your judgment on this one

@mgummelt
Copy link

mgummelt commented May 22, 2017

It looks like Standalone gets around this by constructing a raw java command instead of a spark-class command. Will YARN be using this new SPARK_EXECUTOR_CLASSPATH variable as well?

I don't really see why we would have deprecated SPARK_CLASSPATH only to introduce SPARK_EXECUTOR_CLASSPATH, but I'm fine with this change.

@vanzin
Copy link
Contributor Author

vanzin commented May 22, 2017

Neither standalone nor YARN use env variables to create the executor's classpath, so I guess that's why Mesos was missed. I'd rather Mesos stop using env variables too, but that's a bigger change, and I want to get this in 2.2.0 to avoid a pretty user-visible regression.

@mgummelt
Copy link

mgummelt commented May 22, 2017 •
edited
Loading

It looks like YARN just sets the generic java CLASSPATH env var. Maybe Mesos should do that too, in the future.

@vanzin
Copy link
Contributor Author

vanzin commented May 22, 2017

YARN does not run spark-class, Mesos does. And spark-class does not honor CLASSPATH, IIRC.

@vanzin
Copy link
Contributor Author

vanzin commented May 22, 2017

I'm gonna merge this to unblock 2.2; if there's a desire to clean up this code later, we can do it separately.

asfgit pushed a commit that referenced this pull request May 22, 2017
[SPARK-20814][MESOS] Restore support for spark.executor.extraClassPath.
d8328d8 
Restore code that was removed as part of SPARK-17979, but instead of
using the deprecated env variable name to propagate the class path, use
a new one.

Verified by running "./bin/spark-class o.a.s.executor.CoarseGrainedExecutorBackend"
manually.

Author: Marcelo Vanzin <vanzin@cloudera.com>

Closes #18037 from vanzin/SPARK-20814.

(cherry picked from commit df64fa7)
Signed-off-by: Marcelo Vanzin <vanzin@cloudera.com>
@vanzin vanzin closed this May 22, 2017
@vanzin vanzin deleted the SPARK-20814 branch May 22, 2017 19:36
asfgit pushed a commit that referenced this pull request May 22, 2017
[SPARK-20814][MESOS] Restore support for spark.executor.extraClassPath.
df64fa7 
Restore code that was removed as part of SPARK-17979, but instead of
using the deprecated env variable name to propagate the class path, use
a new one.

Verified by running "./bin/spark-class o.a.s.executor.CoarseGrainedExecutorBackend"
manually.

Author: Marcelo Vanzin <vanzin@cloudera.com>

Closes #18037 from vanzin/SPARK-20814.
lycplus pushed a commit to lycplus/spark that referenced this pull request May 24, 2017
@lycplus
[SPARK-20814][MESOS] Restore support for spark.executor.extraClassPath.
7b5a638 
Restore code that was removed as part of SPARK-17979, but instead of
using the deprecated env variable name to propagate the class path, use
a new one.

Verified by running "./bin/spark-class o.a.s.executor.CoarseGrainedExecutorBackend"
manually.

Author: Marcelo Vanzin <vanzin@cloudera.com>

Closes apache#18037 from vanzin/SPARK-20814.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@srowen srowen srowen approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@vanzin @SparkQA @mgummelt @srowen

Comments